Skip to content

chore: ensure consistent use of semicolons#11170

Merged
kgryte merged 2 commits intodevelopfrom
philipp/fix-addon-semicolons
Mar 27, 2026
Merged

chore: ensure consistent use of semicolons#11170
kgryte merged 2 commits intodevelopfrom
philipp/fix-addon-semicolons

Conversation

@Planeshifter
Copy link
Copy Markdown
Member

@Planeshifter Planeshifter commented Mar 27, 2026 •
edited
Loading

Description

What is the purpose of this pull request?

This pull request:

  • Removes trailing semicolons from 11 addon.c files that had them after NAPI module registration macro invocations, inconsistent with the other ~783 files.

The macros (e.g., STDLIB_MATH_BASE_NAPI_MODULE_D_D) expand to complete function definitions ending with NAPI_MODULE(), so a trailing semicolon is unnecessary. While functionally harmless (it produces an empty declaration at file scope), these 11 files were inconsistent with the rest of the codebase.

Related Issues

Does this pull request have any related issues?

No.

Questions

Any questions for reviewers of this pull request?

No.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

No.

Checklist

Please ensure the following tasks are completed before submitting this pull request.

AI Assistance

When authoring the changes proposed in this PR, did you use any kind of AI assistance?

  • Yes
  • No

If you answered "yes" above, how did you use AI assistance?

  • Code generation (e.g., when writing an implementation or fixing a bug)
  • Test/benchmark generation
  • Documentation (including examples)
  • Research and understanding

Disclosure

If you answered "yes" to using AI assistance, please provide a short disclosure indicating how you used AI assistance. This helps reviewers determine how much scrutiny to apply when reviewing your contribution. Example disclosures: "This PR was written primarily by Claude Code." or "I consulted ChatGPT to understand the codebase, but the proposed changes were fully authored manually by myself.".

This PR was written primarily by Claude Code. Claude audited all ~800 addon.c files for semicolon inconsistencies, identified the 11 violations, and applied the fixes.

@stdlib-js/reviewers

@kgryte
Copy link
Copy Markdown
Member

kgryte commented Mar 27, 2026

@Planeshifter I am not actually sure the proposed changes are desired. See https://gcc.gnu.org/onlinedocs/cpp/Swallowing-the-Semicolon.html.

@kgryte
Copy link
Copy Markdown
Member

kgryte commented Mar 27, 2026

Also, your OP is not correct. Most contributors have never seen the macro definitions, so the auto-generated rationale is misleading.

The semicolons were added in the examples explicitly because of what is discussed in the GCC guide. Namely, because these macros look like function calls, it can minimize confusion to treat them like function calls.

So, I would argue that the docs are actually correct.

Now, that leaves whether it is "necessary" to include a trailing semicolon in, say, addon.c files for unary math functions. If the dominant pattern is no, that is fine there because no additional code follows the macro call.

@Planeshifter
chore: remove errant trailing semicolons after NAPI module macros
4ac7969 
Remove trailing semicolons from NAPI module registration macro
invocations in 11 addon.c files to match the established codebase
convention used by the other ~783 files.

The macros (e.g., STDLIB_MATH_BASE_NAPI_MODULE_D_D) expand to
complete function definitions ending with NAPI_MODULE(), so a
trailing semicolon is unnecessary. While functionally harmless,
these were inconsistent with the rest of the codebase.
@Planeshifter Planeshifter force-pushed the philipp/fix-addon-semicolons branch from a5bfde3 to 4ac7969 Compare March 27, 2026 06:04
@kgryte
Copy link
Copy Markdown
Member

kgryte commented Mar 27, 2026

@Planeshifter See, e.g., https://github.com/stdlib-js/stdlib/blob/develop/lib/node_modules/%40stdlib/blas/ext/base/dapx/src/addon.c#L36 where we include semicolons, and for good reason, because when you stack macro calls, a semicolon can prevent unintended "merging" of macro contents.

@kgryte
Copy link
Copy Markdown
Member

kgryte commented Mar 27, 2026 •
edited
Loading

This is a decent example of a scenario where it is beneficial to have a bit more understanding of what is going on before accepting LLM output.

@Planeshifter
Copy link
Copy Markdown
Member Author

Planeshifter commented Mar 27, 2026

PR was not ready for review and has meanwhile been updated.

@Planeshifter Planeshifter marked this pull request as ready for review March 27, 2026 06:13
@stdlib-bot stdlib-bot added the Needs Review A pull request which needs code review. label Mar 27, 2026
@stdlib-bot
Copy link
Copy Markdown
Contributor

stdlib-bot commented Mar 27, 2026 •
edited
Loading

Coverage Report

Package Statements Branches Functions Lines
math/base/special/erfcx $\color{red}1368/1443$
$\color{green}+0.00%$ 		$\color{green}95/95$
$\color{green}+0.00%$ 		$\color{red}79/104$
$\color{green}+0.00%$ 		$\color{red}1368/1443$
$\color{green}+0.00%$
stats/base/dists/arcsine/median $\color{green}177/177$
$\color{green}+0.00%$ 		$\color{green}7/7$
$\color{green}+0.00%$ 		$\color{green}2/2$
$\color{green}+0.00%$ 		$\color{green}177/177$
$\color{green}+0.00%$
stats/base/dists/binomial/kurtosis $\color{green}178/178$
$\color{green}+0.00%$ 		$\color{green}12/12$
$\color{green}+0.00%$ 		$\color{green}2/2$
$\color{green}+0.00%$ 		$\color{green}178/178$
$\color{green}+0.00%$
stats/base/dists/binomial/mean $\color{green}176/176$
$\color{green}+0.00%$ 		$\color{green}12/12$
$\color{green}+0.00%$ 		$\color{green}2/2$
$\color{green}+0.00%$ 		$\color{green}176/176$
$\color{green}+0.00%$
stats/base/dists/binomial/median $\color{green}178/178$
$\color{green}+0.00%$ 		$\color{green}12/12$
$\color{green}+0.00%$ 		$\color{green}2/2$
$\color{green}+0.00%$ 		$\color{green}178/178$
$\color{green}+0.00%$
stats/base/dists/binomial/mgf $\color{green}326/326$
$\color{green}+0.00%$ 		$\color{green}25/25$
$\color{green}+0.00%$ 		$\color{green}4/4$
$\color{green}+0.00%$ 		$\color{green}326/326$
$\color{green}+0.00%$
stats/base/dists/binomial/mode $\color{green}177/177$
$\color{green}+0.00%$ 		$\color{green}12/12$
$\color{green}+0.00%$ 		$\color{green}2/2$
$\color{green}+0.00%$ 		$\color{green}177/177$
$\color{green}+0.00%$
stats/base/dists/binomial/skewness $\color{green}177/177$
$\color{green}+0.00%$ 		$\color{green}12/12$
$\color{green}+0.00%$ 		$\color{green}2/2$
$\color{green}+0.00%$ 		$\color{green}177/177$
$\color{green}+0.00%$
stats/base/dists/binomial/stdev $\color{green}177/177$
$\color{green}+0.00%$ 		$\color{green}12/12$
$\color{green}+0.00%$ 		$\color{green}2/2$
$\color{green}+0.00%$ 		$\color{green}177/177$
$\color{green}+0.00%$
stats/base/dists/binomial/variance $\color{green}176/176$
$\color{green}+0.00%$ 		$\color{green}12/12$
$\color{green}+0.00%$ 		$\color{green}2/2$
$\color{green}+0.00%$ 		$\color{green}176/176$
$\color{green}+0.00%$
stats/base/dists/planck/mean $\color{green}170/170$
$\color{green}+0.00%$ 		$\color{green}8/8$
$\color{green}+0.00%$ 		$\color{green}2/2$
$\color{green}+0.00%$ 		$\color{green}170/170$
$\color{green}+0.00%$

The above coverage report was generated for the changes in this PR.

@Planeshifter Planeshifter changed the title chore: remove errant trailing semicolons after NAPI module macros chore: remove errant trailing semicolons Mar 27, 2026
@Planeshifter Planeshifter requested a review from kgryte March 27, 2026 06:16
@github-actions github-actions bot mentioned this pull request Mar 27, 2026
Open
kgryte
kgryte reviewed Mar 27, 2026
View reviewed changes
@kgryte
Apply suggestion
2fbb353 
Signed-off-by: Athan <kgryte@gmail.com>
kgryte
kgryte approved these changes Mar 27, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@kgryte kgryte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kgryte kgryte changed the title chore: remove errant trailing semicolons chore: ensure consistent use of semicolons Mar 27, 2026
@kgryte kgryte removed the Needs Review A pull request which needs code review. label Mar 27, 2026
@kgryte kgryte merged commit 8448f5a into develop Mar 27, 2026
17 checks passed
@kgryte kgryte deleted the philipp/fix-addon-semicolons branch March 27, 2026 07:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kgryte kgryte kgryte approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Planeshifter @kgryte @stdlib-bot